Skip to content

feat: Add --show-policy-docs-link flag (default: true)#3173

Open
dheerajodha wants to merge 1 commit intoconforma:mainfrom
dheerajodha:EC-1603
Open

feat: Add --show-policy-docs-link flag (default: true)#3173
dheerajodha wants to merge 1 commit intoconforma:mainfrom
dheerajodha:EC-1603

Conversation

@dheerajodha
Copy link
Contributor

@dheerajodha dheerajodha commented Mar 12, 2026

Summary

Adds --show-policy-docs-link flag to control policy documentation links in validation output. Defaults to false for clean demo output.

Why

Automatic link display cluttered demos. As noted in review: "for demos it's better if the default is false, otherwise every example needs to include the flag."

Changes

  • Added persistent flag on validate command (inherited by all subcommands)
  • Replaced auto-detection logic with flag reads in image.go, input.go, vsa.go
  • Fixed tests to properly initialize flags
  • Updated snapshots (~600 lines removed)

Usage

# Default - clean output
ec validate image --image app:v1 --policy policy.yaml                                                                                                                                                                                       
                                                                                                                                                                                                                                            
# Opt-in for CI/production                                                                                                                                                                                                                  
ec validate image --image app:v1 --policy policy.yaml --show-policy-docs-link=true                                                                                                                                                          

Resolves

EC-1603

@qodo-code-review
Copy link
Contributor

Review Summary by Qodo

Add --show-policy-docs-link flag to control policy documentation visibility
✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add --show-policy-docs-link persistent flag to control policy documentation link visibility
• Flag defaults to true, maintaining backward compatibility with existing behavior
• Propagate flag through all validate subcommands (image, input, vsa) and report generation
• Update templates and format options to conditionally render documentation link based on flag
Diagram
flowchart LR
  A["validate command"] -->|"persistent flag"| B["--show-policy-docs-link<br/>default: true"]
  B -->|"inherited by"| C["image subcommand"]
  B -->|"inherited by"| D["input subcommand"]
  B -->|"inherited by"| E["vsa subcommand"]
  C -->|"passes to"| F["ReportData"]
  D -->|"passes to"| F
  E -->|"passes to"| F
  F -->|"controls rendering"| G["Documentation link<br/>in output"]
Loading

Grey Divider

File Changes

1. cmd/validate/validate.go ✨ Enhancement +1/-0

Add persistent flag definition for policy docs link

cmd/validate/validate.go


2. cmd/validate/image.go ✨ Enhancement +9/-7

Retrieve and pass flag to report data

cmd/validate/image.go


3. cmd/validate/input.go ✨ Enhancement +3/-2

Retrieve and pass flag to report and format options

cmd/validate/input.go


View more (14)
4. cmd/validate/vsa.go ✨ Enhancement +15/-9

Retrieve flag and pass to fallback report data

cmd/validate/vsa.go


5. internal/validate/report.go ✨ Enhancement +12/-9

Add flag field to ReportData struct

internal/validate/report.go


6. internal/applicationsnapshot/report.go ✨ Enhancement +29/-26

Add flag field and pass through report creation

internal/applicationsnapshot/report.go


7. internal/input/report.go ✨ Enhancement +22/-20

Add flag field and pass through report creation

internal/input/report.go


8. internal/format/target.go ✨ Enhancement +11/-2

Add flag field to Options struct and parsing logic

internal/format/target.go


9. internal/applicationsnapshot/templates/text_report.tmpl ✨ Enhancement +1/-1

Conditionally render documentation link based on flag

internal/applicationsnapshot/templates/text_report.tmpl


10. internal/input/templates/text_report.tmpl ✨ Enhancement +1/-1

Conditionally render documentation link based on flag

internal/input/templates/text_report.tmpl


11. internal/applicationsnapshot/report_test.go 🧪 Tests +19/-14

Update test calls with new flag parameter

internal/applicationsnapshot/report_test.go


12. internal/input/report_test.go 🧪 Tests +6/-6

Update test calls with new flag parameter

internal/input/report_test.go


13. docs/modules/ROOT/pages/ec_validate.adoc 📝 Documentation +1/-0

Document new flag in validate command

docs/modules/ROOT/pages/ec_validate.adoc


14. docs/modules/ROOT/pages/ec_validate_image.adoc 📝 Documentation +1/-0

Document inherited flag in image subcommand

docs/modules/ROOT/pages/ec_validate_image.adoc


15. docs/modules/ROOT/pages/ec_validate_input.adoc 📝 Documentation +1/-0

Document inherited flag in input subcommand

docs/modules/ROOT/pages/ec_validate_input.adoc


16. docs/modules/ROOT/pages/ec_validate_policy.adoc 📝 Documentation +1/-0

Document inherited flag in policy subcommand

docs/modules/ROOT/pages/ec_validate_policy.adoc


17. docs/modules/ROOT/pages/ec_validate_vsa.adoc 📝 Documentation +1/-0

Document inherited flag in vsa subcommand

docs/modules/ROOT/pages/ec_validate_vsa.adoc


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 12, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Fallback link always hidden 🐞 Bug ✓ Correctness
Description
In VSA text output, the fallback validate-image section omits the policy documentation link even
when --show-policy-docs-link=true (default), because the fallback text capture builds format.Options
without ShowPolicyDocsLink and applicationsnapshot.Report.WriteAll overwrites the report setting
with the default false option. The new template guard added in this PR then prevents the link from
rendering in that fallback section.
Code

internal/applicationsnapshot/templates/text_report.tmpl[25]

+{{- if and $r.ShowPolicyDocsLink (or (gt $t.Failures 0) (and (gt $t.Warnings 0) $r.ShowWarnings)) -}}
Evidence
The PR changes the text template to only render the documentation link when
Report.ShowPolicyDocsLink is true. In the VSA text path, the fallback report *does* get
ShowPolicyDocsLink from the persistent flag, but captureFallbackText creates a TargetParser with
default options that omit ShowPolicyDocsLink, so it remains false; then
applicationsnapshot.Report.WriteAll applies (overwrites) the report’s ShowPolicyDocsLink from those
target options. This makes the template condition false and suppresses the link regardless of the
flag value.

internal/applicationsnapshot/templates/text_report.tmpl[25-26]
cmd/validate/validate.go[46-49]
cmd/validate/vsa.go[1101-1110]
cmd/validate/vsa.go[1432-1436]
internal/applicationsnapshot/report.go[170-183]
internal/applicationsnapshot/report.go[266-270]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
VSA text output captures fallback validate-image text via `captureFallbackText`. That function constructs `format.Options` without `ShowPolicyDocsLink`, causing `applicationsnapshot.Report.WriteAll()` to overwrite `Report.ShowPolicyDocsLink` to `false`. Since the text template now requires `ShowPolicyDocsLink` to print the documentation link, the fallback section never shows the link even when `--show-policy-docs-link=true`.

### Issue Context
- The fallback report data correctly includes `ShowPolicyDocsLink`, but it is later overridden by target options.
- `applicationsnapshot.Report.WriteAll()` always applies `target.Options` to the report before rendering.

### Fix Focus Areas
- cmd/validate/vsa.go[1432-1446]

Suggested change: include `ShowPolicyDocsLink: fallbackReport.ShowPolicyDocsLink` in `formatOpts` inside `captureFallbackText`, and consider adding/adjusting a test that asserts the fallback section contains/omits the docs link based on the flag.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

This pull request adds a new CLI flag --show-policy-docs-link to the ec validate command and its subcommands (image, input, vsa, policy). When enabled, it controls whether a policy documentation link appears in output when violations or warnings occur. The flag is threaded through the CLI layer, internal report data structures, and template rendering logic.

Changes

Cohort / File(s) Summary
CLI Command Definitions
cmd/validate/validate.go
Added persistent boolean flag --show-policy-docs-link (default: false) to the validate command.
CLI Flag Propagation (Image Validation)
cmd/validate/image.go
Retrieves the --show-policy-docs-link flag and passes it to validate_utils.ReportData construction.
CLI Flag Propagation (Input Validation)
cmd/validate/input.go
Retrieves the --show-policy-docs-link flag and passes it as a new parameter to input.NewReport.
CLI Flag Propagation (VSA Validation)
cmd/validate/vsa.go
Retrieves the flag, adds showPolicyDocsLink to validateVSAData struct, and threads it through validate_utils.ReportData and applicationsnapshot.NewReport calls.
Test Updates
cmd/validate/vsa_test.go
Updated test cases to initialize the --show-policy-docs-link flag in Cobra command instances.
Application Snapshot Report
internal/applicationsnapshot/report.go, internal/applicationsnapshot/report_test.go
Added ShowPolicyDocsLink bool field to Report struct; extended NewReport signature with corresponding parameter; updated all test invocations to pass the new argument.
Input Report
internal/input/report.go, internal/input/report_test.go
Added ShowPolicyDocsLink bool field to Report struct; extended NewReport signature with corresponding parameter; updated all test invocations to pass the new argument.
Validate Report Infrastructure
internal/validate/report.go
Added ShowPolicyDocsLink bool field to ReportData struct; threaded the flag through applicationsnapshot.NewReport calls.
Template Rendering
internal/applicationsnapshot/templates/text_report.tmpl, internal/input/templates/text_report.tmpl
Updated conditional rendering to gate policy documentation link output on the ShowPolicyDocsLink flag in addition to existing failure/warning conditions.
Documentation
docs/modules/ROOT/pages/ec_validate*.adoc
Added documentation for the new --show-policy-docs-link flag across all validate subcommand pages (validate, validate image, validate input, validate policy, validate vsa), describing it as showing a link to policy documentation when violations or warnings occur (default: false).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims the flag defaults to true, but the actual implementation and description show it defaults to false, creating a factual inconsistency. Update the title to 'feat: Add --show-policy-docs-link flag (default: false)' to accurately reflect the actual implementation.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly describes the changes: adding a --show-policy-docs-link flag to control policy documentation links in validation output, with motivation and usage examples.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dheerajodha
Copy link
Contributor Author

I'm thinking if the flag name is too long 🤔

{{- end -}}

{{- if or (gt $t.Failures 0) (and (gt $t.Warnings 0) $r.ShowWarnings) -}}
{{- if and $r.ShowPolicyDocsLink (or (gt $t.Failures 0) (and (gt $t.Warnings 0) $r.ShowWarnings)) -}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Fallback link always hidden 🐞 Bug ✓ Correctness

In VSA text output, the fallback validate-image section omits the policy documentation link even
when --show-policy-docs-link=true (default), because the fallback text capture builds format.Options
without ShowPolicyDocsLink and applicationsnapshot.Report.WriteAll overwrites the report setting
with the default false option. The new template guard added in this PR then prevents the link from
rendering in that fallback section.
Agent Prompt
### Issue description
VSA text output captures fallback validate-image text via `captureFallbackText`. That function constructs `format.Options` without `ShowPolicyDocsLink`, causing `applicationsnapshot.Report.WriteAll()` to overwrite `Report.ShowPolicyDocsLink` to `false`. Since the text template now requires `ShowPolicyDocsLink` to print the documentation link, the fallback section never shows the link even when `--show-policy-docs-link=true`.

### Issue Context
- The fallback report data correctly includes `ShowPolicyDocsLink`, but it is later overridden by target options.
- `applicationsnapshot.Report.WriteAll()` always applies `target.Options` to the report before rendering.

### Fix Focus Areas
- cmd/validate/vsa.go[1432-1446]

Suggested change: include `ShowPolicyDocsLink: fallbackReport.ShowPolicyDocsLink` in `formatOpts` inside `captureFallbackText`, and consider adding/adjusting a test that asserts the fallback section contains/omits the docs link based on the flag.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
internal/input/report.go (1)

101-125: input.Report still can't honor per-output overrides for this new flag.

ShowPolicyDocsLink is now fixed at NewReport(...) construction time, but WriteAll never applies target.Options before rendering. So the ec validate input path still won't honor output-scoped ?show-... overrides the way applicationsnapshot.Report does.

Possible follow-up
+func (r *Report) applyOptions(opts format.Options) {
+	r.ShowSuccesses = opts.ShowSuccesses
+	r.ShowWarnings = opts.ShowWarnings
+	r.ShowPolicyDocsLink = opts.ShowPolicyDocsLink
+}
+
 // WriteAll writes the report to all the given targets.
 func (r Report) WriteAll(targets []string, p format.TargetParser) (allErrors error) {
 	if len(targets) == 0 {
 		targets = append(targets, Text)
 	}
 	for _, targetName := range targets {
 		target, err := p.Parse(targetName)
 		if err != nil {
 			allErrors = errors.Join(allErrors, err)
 			continue
 		}
+
+		r.applyOptions(target.Options)

 		data, err := r.toFormat(target.Format)
 		if err != nil {
 			allErrors = errors.Join(allErrors, err)
 			continue
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/input/report.go` around lines 101 - 125, NewReport currently fixes
ShowPolicyDocsLink at construction time so per-output flags in target.Options
are ignored; update the flow so NewReport does not permanently bake the show
flag (or leaves it unset/default) and ensure WriteAll (the renderer) applies
each target.Options prior to rendering—mirror applicationsnapshot.Report's
behavior by reading target.Options for ShowPolicyDocsLink inside WriteAll (or
setting report.ShowPolicyDocsLink from target.Options just before render) so
output-scoped ?show-... overrides are honored.
internal/input/report_test.go (1)

39-40: Add a regression test for showPolicyDocsLink=false on the input report path.

All of the updated NewReport(...) calls pass true for the new flag, and none of the text-output assertions cover warnings or violations with the flag disabled. A regression that still prints the docs link for ec validate input --show-policy-docs-link=false would slip through here.

Also applies to: 129-130, 235-236, 321-322, 360-361, 385-386

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/input/report_test.go` around lines 39 - 40, Add regression tests in
internal/input/report_test.go that call NewReport with the showPolicyDocsLink
flag set to false (i.e., change or add cases where NewReport(inputs, testPolicy,
nil, false, false, true) is used) and assert that the generated text output for
warnings/violations does NOT contain the policy docs link; update the other
affected test invocations (the similar calls at the other noted spots) to
include a corresponding test case with showPolicyDocsLink=false to ensure the
docs link is suppressed.
internal/applicationsnapshot/report_test.go (1)

1036-1059: Please cover the ShowPolicyDocsLink=false branch too.

These cases prove the link appears when the flag is enabled, but not that it disappears when warnings or violations exist and the flag is disabled. That suppression path is the user-visible behavior this flag adds.

Also applies to: 1065-1087, 1093-1123, 1129-1168

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/applicationsnapshot/report_test.go` around lines 1036 - 1059, Add a
negative test case that asserts the policy docs link is suppressed when
ShowPolicyDocsLink is false: construct a Report (same setup as the existing
test) with ShowPolicyDocsLink: false and one Component containing a warning
(evaluator.Result with "code":"warning.policy"), call generateTextReport(&r) and
assert no "https://conforma.dev/docs/policy/" link and that the "For more
information about policy issues" message is absent; mirror this for the other
similar test blocks referenced (lines around generateTextReport usage) to cover
all branches where ShowPolicyDocsLink toggles the link.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/validate/validate.go`:
- Line 48: The flag "show-policy-docs-link" is registered on the parent validate
command (validateCmd) but only used by specific subcommands (validate image,
validate input, validate vsa) while ValidatePolicyCmd never reads it; either
move the flag registration off validateCmd and register it only on the
subcommands that use it (the image/input/vsa command builders where flags are
currently read), or add logic in the ValidatePolicyCmd handler to read and honor
the "show-policy-docs-link" persistent flag (use the same flag key) so ec
validate policy --show-policy-docs-link behaves consistently. Ensure you update
the flag registration location(s) and the code path that reads the flag (the
existing readers used by image/input/vsa or the ValidatePolicyCmd) so there are
no unused persistent flags and behavior is consistent.

---

Nitpick comments:
In `@internal/applicationsnapshot/report_test.go`:
- Around line 1036-1059: Add a negative test case that asserts the policy docs
link is suppressed when ShowPolicyDocsLink is false: construct a Report (same
setup as the existing test) with ShowPolicyDocsLink: false and one Component
containing a warning (evaluator.Result with "code":"warning.policy"), call
generateTextReport(&r) and assert no "https://conforma.dev/docs/policy/" link
and that the "For more information about policy issues" message is absent;
mirror this for the other similar test blocks referenced (lines around
generateTextReport usage) to cover all branches where ShowPolicyDocsLink toggles
the link.

In `@internal/input/report_test.go`:
- Around line 39-40: Add regression tests in internal/input/report_test.go that
call NewReport with the showPolicyDocsLink flag set to false (i.e., change or
add cases where NewReport(inputs, testPolicy, nil, false, false, true) is used)
and assert that the generated text output for warnings/violations does NOT
contain the policy docs link; update the other affected test invocations (the
similar calls at the other noted spots) to include a corresponding test case
with showPolicyDocsLink=false to ensure the docs link is suppressed.

In `@internal/input/report.go`:
- Around line 101-125: NewReport currently fixes ShowPolicyDocsLink at
construction time so per-output flags in target.Options are ignored; update the
flow so NewReport does not permanently bake the show flag (or leaves it
unset/default) and ensure WriteAll (the renderer) applies each target.Options
prior to rendering—mirror applicationsnapshot.Report's behavior by reading
target.Options for ShowPolicyDocsLink inside WriteAll (or setting
report.ShowPolicyDocsLink from target.Options just before render) so
output-scoped ?show-... overrides are honored.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 27e72ba8-c638-452c-a969-04b6a8761845

📥 Commits

Reviewing files that changed from the base of the PR and between 3ce85d1 and 450de2e.

📒 Files selected for processing (17)
  • cmd/validate/image.go
  • cmd/validate/input.go
  • cmd/validate/validate.go
  • cmd/validate/vsa.go
  • docs/modules/ROOT/pages/ec_validate.adoc
  • docs/modules/ROOT/pages/ec_validate_image.adoc
  • docs/modules/ROOT/pages/ec_validate_input.adoc
  • docs/modules/ROOT/pages/ec_validate_policy.adoc
  • docs/modules/ROOT/pages/ec_validate_vsa.adoc
  • internal/applicationsnapshot/report.go
  • internal/applicationsnapshot/report_test.go
  • internal/applicationsnapshot/templates/text_report.tmpl
  • internal/format/target.go
  • internal/input/report.go
  • internal/input/report_test.go
  • internal/input/templates/text_report.tmpl
  • internal/validate/report.go

}
validateCmd.PersistentFlags().Bool("show-successes", false, "")
validateCmd.PersistentFlags().Bool("show-warnings", true, "")
validateCmd.PersistentFlags().Bool("show-policy-docs-link", true, "Show the policy documentation link when there are failures or warnings")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\nValidatePolicyCmd definition:\n'
rg -nC4 --type=go '\bValidatePolicyCmd\s*\(' cmd internal

printf '\nshow-policy-docs-link / ShowPolicyDocsLink references:\n'
rg -nC3 --type=go 'show-policy-docs-link|ShowPolicyDocsLink' cmd internal

printf '\nPolicy-doc-link template/report references:\n'
rg -nC3 'conforma\.dev/docs/policy|policy documentation' cmd internal

Repository: conforma/cli

Length of output: 23965


Move show-policy-docs-link flag to individual subcommands or add support in validate policy.

The flag is registered on the parent validate command (line 48), making it available to all subcommands. However, only validate image, validate input, and validate vsa actually read and use this flag. ValidatePolicyCmd never reads it, so ec validate policy --show-policy-docs-link=... becomes a silent no-op for users.

Either:

  • Register the flag only on subcommands that use it (image, input, vsa), or
  • Add support to consume it in the policy validation command.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/validate/validate.go` at line 48, The flag "show-policy-docs-link" is
registered on the parent validate command (validateCmd) but only used by
specific subcommands (validate image, validate input, validate vsa) while
ValidatePolicyCmd never reads it; either move the flag registration off
validateCmd and register it only on the subcommands that use it (the
image/input/vsa command builders where flags are currently read), or add logic
in the ValidatePolicyCmd handler to read and honor the "show-policy-docs-link"
persistent flag (use the same flag key) so ec validate policy
--show-policy-docs-link behaves consistently. Ensure you update the flag
registration location(s) and the code path that reads the flag (the existing
readers used by image/input/vsa or the ValidatePolicyCmd) so there are no unused
persistent flags and behavior is consistent.

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 75.86207% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/validate/vsa.go 0.00% 10 Missing ⚠️
internal/format/target.go 20.00% 4 Missing ⚠️
Flag Coverage Δ
acceptance 54.87% <75.86%> (+<0.01%) ⬆️
generative 18.12% <0.00%> (-0.03%) ⬇️
integration 26.97% <17.24%> (-0.02%) ⬇️
unit 68.64% <75.86%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/validate/image.go 91.36% <100.00%> (+0.04%) ⬆️
cmd/validate/input.go 94.15% <100.00%> (+0.03%) ⬆️
cmd/validate/validate.go 100.00% <100.00%> (ø)
internal/applicationsnapshot/report.go 88.35% <100.00%> (+0.12%) ⬆️
internal/input/report.go 88.34% <100.00%> (+0.11%) ⬆️
internal/validate/report.go 97.50% <100.00%> (+0.06%) ⬆️
internal/format/target.go 79.54% <20.00%> (-7.64%) ⬇️
cmd/validate/vsa.go 35.84% <0.00%> (-0.12%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dheerajodha dheerajodha marked this pull request as draft March 12, 2026 16:16
@simonbaird
Copy link
Member

I know we generally prefer not to change existing behavior when adding a new flag, but for demos it's better if the default is false, otherwise every example in a demo needs to include the flag. Wdyt?

Also:

  • Did you consider any of the other options suggested in the Jira?
  • Jira ref seems to be missing from commit and PR.

@dheerajodha
Copy link
Contributor Author

I know we generally prefer not to change existing behavior when adding a new flag, but for demos it's better if the default is false, otherwise every example in a demo needs to include the flag. Wdyt?

Also:

  • Did you consider any of the other options suggested in the Jira?

I thought changing the CRD definition for just a docs info from the CLI would be overkill, so I settled for the flag idea. But my latest commit changes the behaviour from using a logic that checks if the user is passing snapshot, then they'd be shown the docs link. But you raised a good point in the standup, so I'm thinking of switching back to the flag idea, and then there's the CRD-based plan. What are your thoughts?

  • Jira ref seems to be missing from commit and PR.

Sorry yes, updated now. I will update the PR title and description once we settle on the approach.

Give users control over policy documentation links in validation reports.
Defaults to false to keep demo output clean—production CI can opt in with
--show-policy-docs-link=true when they want the help link.

Why default to false? Your colleague nailed it: "for demos it's better if
the default is false, otherwise every example needs to include the flag."
Nobody wants documentation URLs cluttering their quick validation examples.

Implementation:
- Added persistent flag on parent validate command (all subcommands inherit)
- Flag controls display of https://conforma.dev/docs/policy/ link
- Link only appears when violations/warnings exist
- Updated tests to properly initialize flags
- Updated snapshots for new default behavior

Usage:
  # Default - clean output for demos
  ec validate image --image <img> --policy <policy>

  # Opt-in for production/CI
  ec validate image --image <img> --policy <policy> --show-policy-docs-link=true

resolves: EC-1603

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@dheerajodha dheerajodha marked this pull request as ready for review March 25, 2026 13:03
@qodo-code-review
Copy link
Contributor

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Add --show-policy-docs-link flag to control policy documentation link visibility

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add --show-policy-docs-link persistent flag to control policy documentation link display
• Default value changed to false for cleaner demo output
• Flag inherited by all validate subcommands (image, input, vsa)
• Updated report structures and templates to respect the new flag setting
• Updated tests and documentation to reflect new parameter
Diagram
flowchart LR
  A["validate command"] -->|"adds persistent flag"| B["--show-policy-docs-link"]
  B -->|"inherited by"| C["validate image"]
  B -->|"inherited by"| D["validate input"]
  B -->|"inherited by"| E["validate vsa"]
  C -->|"passes to"| F["ReportData"]
  D -->|"passes to"| F
  E -->|"passes to"| F
  F -->|"controls display in"| G["text_report.tmpl"]
  G -->|"shows/hides"| H["policy documentation link"]
Loading

Grey Divider

File Changes

1. cmd/validate/validate.go ✨ Enhancement +1/-0

Add show-policy-docs-link persistent flag definition

cmd/validate/validate.go


2. cmd/validate/image.go ✨ Enhancement +9/-7

Read and pass show-policy-docs-link flag to report

cmd/validate/image.go


3. cmd/validate/input.go ✨ Enhancement +2/-1

Read and pass show-policy-docs-link flag to report

cmd/validate/input.go


View more (14)
4. cmd/validate/vsa.go ✨ Enhancement +15/-9

Add show-policy-docs-link field and flag handling

cmd/validate/vsa.go


5. cmd/validate/vsa_test.go 🧪 Tests +4/-0

Initialize show-policy-docs-link flag in tests

cmd/validate/vsa_test.go


6. internal/applicationsnapshot/report.go ✨ Enhancement +28/-26

Add ShowPolicyDocsLink field to Report struct

internal/applicationsnapshot/report.go


7. internal/applicationsnapshot/report_test.go 🧪 Tests +19/-14

Update test calls with new ShowPolicyDocsLink parameter

internal/applicationsnapshot/report_test.go


8. internal/input/report.go ✨ Enhancement +22/-20

Add ShowPolicyDocsLink field to Report struct

internal/input/report.go


9. internal/input/report_test.go 🧪 Tests +6/-6

Update test calls with new ShowPolicyDocsLink parameter

internal/input/report_test.go


10. internal/validate/report.go ✨ Enhancement +9/-7

Add ShowPolicyDocsLink field to ReportData struct

internal/validate/report.go


11. internal/applicationsnapshot/templates/text_report.tmpl ✨ Enhancement +1/-1

Conditionally display policy docs link based on flag

internal/applicationsnapshot/templates/text_report.tmpl


12. internal/input/templates/text_report.tmpl ✨ Enhancement +1/-1

Conditionally display policy docs link based on flag

internal/input/templates/text_report.tmpl


13. docs/modules/ROOT/pages/ec_validate.adoc 📝 Documentation +1/-0

Document show-policy-docs-link flag option

docs/modules/ROOT/pages/ec_validate.adoc


14. docs/modules/ROOT/pages/ec_validate_image.adoc 📝 Documentation +1/-0

Document show-policy-docs-link flag option

docs/modules/ROOT/pages/ec_validate_image.adoc


15. docs/modules/ROOT/pages/ec_validate_input.adoc 📝 Documentation +1/-0

Document show-policy-docs-link flag option

docs/modules/ROOT/pages/ec_validate_input.adoc


16. docs/modules/ROOT/pages/ec_validate_policy.adoc 📝 Documentation +1/-0

Document show-policy-docs-link flag option

docs/modules/ROOT/pages/ec_validate_policy.adoc


17. docs/modules/ROOT/pages/ec_validate_vsa.adoc 📝 Documentation +1/-0

Document show-policy-docs-link flag option

docs/modules/ROOT/pages/ec_validate_vsa.adoc


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 25, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Wrong flag default 🐞 Bug ✓ Correctness ⭐ New
Description
The new persistent flag --show-policy-docs-link is registered with default=false, so the
documentation link will be hidden by default and validate output changes from prior behavior (and
from the PR description).
Code

cmd/validate/validate.go[R46-49]

	validateCmd.PersistentFlags().Bool("show-successes", false, "")
	validateCmd.PersistentFlags().Bool("show-warnings", true, "")
+	validateCmd.PersistentFlags().Bool("show-policy-docs-link", false, "Show link to policy documentation in output when there are violations or warnings")
	return validateCmd
Evidence
The validate root command registers the flag with default false. The text templates were changed to
only render the docs link when ShowPolicyDocsLink is true, so with the current default the link will
never show unless explicitly enabled—contradicting the PR description and intended backward
compatibility.

Code evidence:
- Flag default is false in validate command.
- Link rendering is gated by ShowPolicyDocsLink in both snapshot and input text templates.
- Docs pages also show Default: false, reinforcing the behavior mismatch with the PR description.

cmd/validate/validate.go[41-50]
internal/applicationsnapshot/templates/text_report.tmpl[15-27]
internal/input/templates/text_report.tmpl[15-27]
docs/modules/ROOT/pages/ec_validate.adoc[1-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`--show-policy-docs-link` is intended to default to `true` (backward compatible), but it is currently registered with a default of `false`. Because the text report templates now require `ShowPolicyDocsLink` to be true to render the docs link, the docs link is suppressed by default.

### Issue Context
This impacts validate subcommands inheriting the persistent flag (`validate image`, `validate input`, `validate vsa` fallback reports).

### Fix Focus Areas
- cmd/validate/validate.go[46-49]  
 - Change the default value to `true`.
- docs/modules/ROOT/pages/ec_validate.adoc[5-10]  
 - Update displayed default to `true`.
- docs/modules/ROOT/pages/ec_validate_image.adoc[172-177]
- docs/modules/ROOT/pages/ec_validate_input.adoc[76-81]
- docs/modules/ROOT/pages/ec_validate_policy.adoc[37-42]
- docs/modules/ROOT/pages/ec_validate_vsa.adoc[60-65]
 - Update displayed defaults consistently.

### Notes
After changing the default, consider adding/adjusting tests that validate the docs link appears by default when warnings/violations exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Fallback link always hidden 🐞 Bug ✓ Correctness
Description
In VSA text output, the fallback validate-image section omits the policy documentation link even
when --show-policy-docs-link=true (default), because the fallback text capture builds format.Options
without ShowPolicyDocsLink and applicationsnapshot.Report.WriteAll overwrites the report setting
with the default false option. The new template guard added in this PR then prevents the link from
rendering in that fallback section.
Code

internal/applicationsnapshot/templates/text_report.tmpl[25]

+{{- if and $r.ShowPolicyDocsLink (or (gt $t.Failures 0) (and (gt $t.Warnings 0) $r.ShowWarnings)) -}}
Evidence
The PR changes the text template to only render the documentation link when
Report.ShowPolicyDocsLink is true. In the VSA text path, the fallback report *does* get
ShowPolicyDocsLink from the persistent flag, but captureFallbackText creates a TargetParser with
default options that omit ShowPolicyDocsLink, so it remains false; then
applicationsnapshot.Report.WriteAll applies (overwrites) the report’s ShowPolicyDocsLink from those
target options. This makes the template condition false and suppresses the link regardless of the
flag value.

internal/applicationsnapshot/templates/text_report.tmpl[25-26]
cmd/validate/validate.go[46-49]
cmd/validate/vsa.go[1101-1110]
cmd/validate/vsa.go[1432-1436]
internal/applicationsnapshot/report.go[170-183]
internal/applicationsnapshot/report.go[266-270]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
VSA text output captures fallback validate-image text via `captureFallbackText`. That function constructs `format.Options` without `ShowPolicyDocsLink`, causing `applicationsnapshot.Report.WriteAll()` to overwrite `Report.ShowPolicyDocsLink` to `false`. Since the text template now requires `ShowPolicyDocsLink` to print the documentation link, the fallback section never shows the link even when `--show-policy-docs-link=true`.
### Issue Context
- The fallback report data correctly includes `ShowPolicyDocsLink`, but it is later overridden by target options.
- `applicationsnapshot.Report.WriteAll()` always applies `target.Options` to the report before rendering.
### Fix Focus Areas
- cmd/validate/vsa.go[1432-1446]
Suggested change: include `ShowPolicyDocsLink: fallbackReport.ShowPolicyDocsLink` in `formatOpts` inside `captureFallbackText`, and consider adding/adjusting a test that asserts the fallback section contains/omits the docs link based on the flag.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Hidden flag retrieval errors 🐞 Bug ⛯ Reliability ⭐ New
Description
The new flag value is retrieved via cmd.Flags().GetBool("show-policy-docs-link") with the returned
error ignored, which can silently fall back to false when the flag isn’t registered (notably in unit
tests or reused helpers).
Code

cmd/validate/vsa.go[R270-272]

+	// Get show-policy-docs-link flag value
+	data.showPolicyDocsLink, _ = cmd.Flags().GetBool("show-policy-docs-link")
+
Evidence
In runValidateVSA (and similarly in validate image/input) the code discards the error from
GetBool. Cobra returns an error when a flag doesn’t exist on that command; ignoring it makes
behavior silently default to false. The repository’s own tests had to manually add the flag to a
bare cobra.Command, which demonstrates this failure mode in non-standard command constructions.

cmd/validate/vsa.go[255-285]
cmd/validate/input.go[116-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Flag lookup errors for `show-policy-docs-link` are ignored, which can silently change behavior to `false` when the flag isn’t registered on the command.

### Issue Context
This is most likely to bite in tests, helper invocations, or if `runValidateVSA` is called with a synthetic `cobra.Command` lacking inherited persistent flags.

### Fix Focus Areas
- cmd/validate/vsa.go[270-272]
- cmd/validate/image.go[339-341]
- cmd/validate/input.go[122-125]

### Suggested approach
- Capture and return the error (or at least log/debug it) if `GetBool` fails.
- Alternatively, use `cmd.InheritedFlags().Lookup(...)` / `cmd.Flags().Lookup(...)` and explicitly define a safe fallback only when you can prove the flag is intentionally absent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/applicationsnapshot/report.go (1)

132-165: Replace the bool train with a small options struct.

NewReport(..., showSuccesses, showWarnings, showPolicyDocsLink, expansion) is already yielding opaque call sites like NewReport(..., true, true, true, nil) in internal/applicationsnapshot/report_test.go. That is easy to misorder and gets worse each time a report toggle is added. Worth switching to a ReportOptions struct here, and applying the same shape to internal/input.NewReport while this flag is still new.

As per coding guidelines, Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/applicationsnapshot/report.go` around lines 132 - 165, Introduce a
small ReportOptions struct (e.g., type ReportOptions struct { ShowSuccesses,
ShowWarnings, ShowPolicyDocsLink bool; Expansion *ExpansionInfo }) and replace
the boolean flags + expansion parameter in NewReport with a single opts
ReportOptions parameter; update NewReport to read opts.ShowSuccesses,
opts.ShowWarnings, opts.ShowPolicyDocsLink and opts.Expansion when building the
Report, and adjust all call sites (notably internal/input.NewReport and
internal/applicationsnapshot/report_test.go) to pass a ReportOptions value
instead of positional booleans so callers become explicit and order-safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/modules/ROOT/pages/ec_validate_input.adoc`:
- Line 79: The --show-policy-docs-link flag currently has a default of false;
change its registration in the validate command (the flag registration that
defines "show-policy-docs-link" in validate.go) to use Default: true (or set the
Bool flag default to true) and update the documentation entry that shows the
default value (the line displaying "--show-policy-docs-link:: ... (Default:
false)" in the ec_validate_input.adoc page) to reflect "(Default: true)". Ensure
the flag variable/name remains unchanged and only the default and doc text are
updated so behavior and help output match.

---

Nitpick comments:
In `@internal/applicationsnapshot/report.go`:
- Around line 132-165: Introduce a small ReportOptions struct (e.g., type
ReportOptions struct { ShowSuccesses, ShowWarnings, ShowPolicyDocsLink bool;
Expansion *ExpansionInfo }) and replace the boolean flags + expansion parameter
in NewReport with a single opts ReportOptions parameter; update NewReport to
read opts.ShowSuccesses, opts.ShowWarnings, opts.ShowPolicyDocsLink and
opts.Expansion when building the Report, and adjust all call sites (notably
internal/input.NewReport and internal/applicationsnapshot/report_test.go) to
pass a ReportOptions value instead of positional booleans so callers become
explicit and order-safe.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c039650f-59aa-499b-b08b-f08c6355cac4

📥 Commits

Reviewing files that changed from the base of the PR and between 450de2e and 2997b87.

⛔ Files ignored due to path filters (9)
  • features/__snapshots__/conftest_test.snap is excluded by !**/*.snap
  • features/__snapshots__/initialize.snap is excluded by !**/*.snap
  • features/__snapshots__/inspect_policy.snap is excluded by !**/*.snap
  • features/__snapshots__/ta_task_validate_image.snap is excluded by !**/*.snap
  • features/__snapshots__/task_validate_image.snap is excluded by !**/*.snap
  • features/__snapshots__/track_bundle.snap is excluded by !**/*.snap
  • features/__snapshots__/validate_image.snap is excluded by !**/*.snap
  • features/__snapshots__/validate_input.snap is excluded by !**/*.snap
  • features/__snapshots__/vsa.snap is excluded by !**/*.snap
📒 Files selected for processing (17)
  • cmd/validate/image.go
  • cmd/validate/input.go
  • cmd/validate/validate.go
  • cmd/validate/vsa.go
  • cmd/validate/vsa_test.go
  • docs/modules/ROOT/pages/ec_validate.adoc
  • docs/modules/ROOT/pages/ec_validate_image.adoc
  • docs/modules/ROOT/pages/ec_validate_input.adoc
  • docs/modules/ROOT/pages/ec_validate_policy.adoc
  • docs/modules/ROOT/pages/ec_validate_vsa.adoc
  • internal/applicationsnapshot/report.go
  • internal/applicationsnapshot/report_test.go
  • internal/applicationsnapshot/templates/text_report.tmpl
  • internal/input/report.go
  • internal/input/report_test.go
  • internal/input/templates/text_report.tmpl
  • internal/validate/report.go
✅ Files skipped from review due to trivial changes (3)
  • docs/modules/ROOT/pages/ec_validate_policy.adoc
  • docs/modules/ROOT/pages/ec_validate_image.adoc
  • cmd/validate/vsa_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • docs/modules/ROOT/pages/ec_validate.adoc
  • cmd/validate/validate.go
  • internal/applicationsnapshot/templates/text_report.tmpl
  • docs/modules/ROOT/pages/ec_validate_vsa.adoc
  • internal/input/report_test.go
  • cmd/validate/vsa.go

--retry-jitter:: randomness factor for backoff calculation (0.0-1.0) (Default: 0.1)
--retry-max-retry:: maximum number of retry attempts (Default: 3)
--retry-max-wait:: maximum wait time between retries (Default: 3s)
--show-policy-docs-link:: Show link to policy documentation in output when there are violations or warnings (Default: false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C2 --type=go 'show-policy-docs-link' cmd/validate
printf '\n'
rg -n -C1 --type=adoc 'show-policy-docs-link' docs/modules/ROOT/pages

Repository: conforma/cli

Length of output: 2464


Update flag default to true for backward compatibility.

The --show-policy-docs-link flag defaults to false in both code (cmd/validate/validate.go:48) and documentation (line 79), but backward compatibility requires it to default to true. This flips the behavior from opt-out to opt-in, breaking existing workflows. Update the flag registration and documentation to use Default: true.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/modules/ROOT/pages/ec_validate_input.adoc` at line 79, The
--show-policy-docs-link flag currently has a default of false; change its
registration in the validate command (the flag registration that defines
"show-policy-docs-link" in validate.go) to use Default: true (or set the Bool
flag default to true) and update the documentation entry that shows the default
value (the line displaying "--show-policy-docs-link:: ... (Default: false)" in
the ec_validate_input.adoc page) to reflect "(Default: true)". Ensure the flag
variable/name remains unchanged and only the default and doc text are updated so
behavior and help output match.

Comment on lines 46 to 49
validateCmd.PersistentFlags().Bool("show-successes", false, "")
validateCmd.PersistentFlags().Bool("show-warnings", true, "")
validateCmd.PersistentFlags().Bool("show-policy-docs-link", false, "Show link to policy documentation in output when there are violations or warnings")
return validateCmd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Wrong flag default 🐞 Bug ✓ Correctness

The new persistent flag --show-policy-docs-link is registered with default=false, so the
documentation link will be hidden by default and validate output changes from prior behavior (and
from the PR description).
Agent Prompt
### Issue description
`--show-policy-docs-link` is intended to default to `true` (backward compatible), but it is currently registered with a default of `false`. Because the text report templates now require `ShowPolicyDocsLink` to be true to render the docs link, the docs link is suppressed by default.

### Issue Context
This impacts validate subcommands inheriting the persistent flag (`validate image`, `validate input`, `validate vsa` fallback reports).

### Fix Focus Areas
- cmd/validate/validate.go[46-49]  
  - Change the default value to `true`.
- docs/modules/ROOT/pages/ec_validate.adoc[5-10]  
  - Update displayed default to `true`.
- docs/modules/ROOT/pages/ec_validate_image.adoc[172-177]
- docs/modules/ROOT/pages/ec_validate_input.adoc[76-81]
- docs/modules/ROOT/pages/ec_validate_policy.adoc[37-42]
- docs/modules/ROOT/pages/ec_validate_vsa.adoc[60-65]
  - Update displayed defaults consistently.

### Notes
After changing the default, consider adding/adjusting tests that validate the docs link appears by default when warnings/violations exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR description to reflect the current beahviour. Recheck :whip:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants